IBX-6631: Enriched TrashItem with removedLocationContentIdMap
#388
Conversation
barw4
added
Improvement
eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php
Outdated
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
| /** | ||
| * @return array<int, int> | ||
| */ | ||
| public function getRemovedLocationContentIdMap(): array |
There was a problem hiding this comment.
In https://github.com/ezsystems/date-based-publisher/pull/267 I see that you are using this map to create content deletion notifications, and perform actual deletions.
Note that Content can have multiple locations, which means the same Content ID might occur in this map multiple times. Is this your intent?
There was a problem hiding this comment.
@Steveb-p Yes, it can appear multiple times. Why would that be a problem? If one schedule entry is deleted, the second one won't be found and hence, won't be deleted. No exception is being thrown.
There was a problem hiding this comment.
If that is the case, then it is ok. I assumed it was a case that was not taken into account and the same action was taken for the same Content ID more than once.
You may consider swapping the map order. By having it be array<ContentID, array<LocationID>> (array with ContentID as keys and an array of LocationIDs as value) you would have a similar effect, with Location IDs being grouped together. Only a suggestion though, of course.
v3.3Related PR: https://github.com/ezsystems/date-based-publisher/pull/267
One assert was removed because crucial values are compared in the same method which makes this assert obsolete. Obviously, it also fails as loaded
TrashItemwill always have an emptyremovedLocationContentIdMapwhether the one built after trashing - won't.Checklist:
$ composer fix-cs).@ezsystems/engineering-team).